[mlir][emitc] Relax hasSideEffect rules for Var/Member
#145011
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-mlir-emitc Author: Kirill Chibisov (kchibisov) ChangesThe load side effect semantic depends on the operation that was used to declare the load's operand. In case of -- Not sure whether the logic of #144990 should be applied here, since the Full diff: https://github.com/llvm/llvm-project/pull/145011.diff 4 Files Affected:
diff --git a/mlir/include/mlir/Dialect/EmitC/IR/EmitC.td b/mlir/include/mlir/Dialect/EmitC/IR/EmitC.td
index 9ecdb74f4d82e..d88407a4d6726 100644
--- a/mlir/include/mlir/Dialect/EmitC/IR/EmitC.td
+++ b/mlir/include/mlir/Dialect/EmitC/IR/EmitC.td
@@ -966,6 +966,10 @@ def EmitC_LoadOp : EmitC_Op<"load", [CExpressionInterface,
let results = (outs AnyType:$result);
let assemblyFormat = "$operand attr-dict `:` type($operand)";
+
+ let extraClassDeclaration = [{
+ bool hasSideEffects();
+ }];
}
def EmitC_MulOp : EmitC_BinaryOp<"mul", []> {
diff --git a/mlir/lib/Dialect/EmitC/IR/EmitC.cpp b/mlir/lib/Dialect/EmitC/IR/EmitC.cpp
index e602210c2dc6c..10e0d931e17fd 100644
--- a/mlir/lib/Dialect/EmitC/IR/EmitC.cpp
+++ b/mlir/lib/Dialect/EmitC/IR/EmitC.cpp
@@ -898,6 +898,20 @@ LogicalResult emitc::VariableOp::verify() {
return verifyInitializationAttribute(getOperation(), getValueAttr());
}
+//===----------------------------------------------------------------------===//
+// VariableOp
+//===----------------------------------------------------------------------===//
+
+bool emitc::LoadOp::hasSideEffects() {
+ auto *defOp = this->getOperand().getDefiningOp();
+ // The load side effect semantic depends on the operation that was used to
+ // declare the load's operand, `VariableOp` and `MemberOp` perform loading
+ // from the stack, thus we may consider them as not having side effect to
+ // make end code more readable by letting those loads to get inlined.
+ return !defOp ||
+ !(llvm::isa<VariableOp>(defOp) || llvm::isa<MemberOp>(defOp));
+}
+
//===----------------------------------------------------------------------===//
// YieldOp
//===----------------------------------------------------------------------===//
diff --git a/mlir/test/Dialect/EmitC/transforms.mlir b/mlir/test/Dialect/EmitC/transforms.mlir
index a38f396dad953..687ebe2cd3612 100644
--- a/mlir/test/Dialect/EmitC/transforms.mlir
+++ b/mlir/test/Dialect/EmitC/transforms.mlir
@@ -135,21 +135,18 @@ func.func @single_result_requirement() -> (i32, i32) {
// CHECK-SAME: %[[VAL_1:.*]]: !emitc.ptr<i32>) -> i1 {
// CHECK: %[[VAL_2:.*]] = "emitc.constant"() <{value = 0 : i64}> : () -> i64
// CHECK: %[[VAL_3:.*]] = "emitc.variable"() <{value = #emitc.opaque<"42">}> : () -> !emitc.lvalue<i32>
-// CHECK: %[[VAL_4:.*]] = emitc.expression : i32 {
-// CHECK: %[[VAL_5:.*]] = load %[[VAL_3]] : <i32>
-// CHECK: yield %[[VAL_5]] : i32
+// CHECK: %[[VAL_4:.*]] = emitc.subscript %[[VAL_1]]{{\[}}%[[VAL_2]]] : (!emitc.ptr<i32>, i64) -> !emitc.lvalue<i32>
+// CHECK: %[[VAL_5:.*]] = emitc.expression : i32 {
+// CHECK: %[[VAL_6:.*]] = load %[[VAL_4]] : <i32>
+// CHECK: yield %[[VAL_6]] : i32
// CHECK: }
-// CHECK: %[[VAL_6:.*]] = emitc.subscript %[[VAL_1]]{{\[}}%[[VAL_2]]] : (!emitc.ptr<i32>, i64) -> !emitc.lvalue<i32>
-// CHECK: %[[VAL_7:.*]] = emitc.expression : i32 {
-// CHECK: %[[VAL_8:.*]] = load %[[VAL_6]] : <i32>
-// CHECK: yield %[[VAL_8]] : i32
+// CHECK: %[[VAL_7:.*]] = emitc.expression : i1 {
+// CHECK: %[[VAL_8:.*]] = load %[[VAL_3]] : <i32>
+// CHECK: %[[VAL_9:.*]] = add %[[VAL_8]], %[[VAL_5]] : (i32, i32) -> i32
+// CHECK: %[[VAL_10:.*]] = cmp lt, %[[VAL_9]], %[[VAL_0]] : (i32, i32) -> i1
+// CHECK: yield %[[VAL_10]] : i1
// CHECK: }
-// CHECK: %[[VAL_9:.*]] = emitc.expression : i1 {
-// CHECK: %[[VAL_10:.*]] = add %[[VAL_4]], %[[VAL_7]] : (i32, i32) -> i32
-// CHECK: %[[VAL_11:.*]] = cmp lt, %[[VAL_10]], %[[VAL_0]] : (i32, i32) -> i1
-// CHECK: yield %[[VAL_11]] : i1
-// CHECK: }
-// CHECK: return %[[VAL_9]] : i1
+// CHECK: return %[[VAL_7]] : i1
// CHECK: }
diff --git a/mlir/test/Target/Cpp/expressions.mlir b/mlir/test/Target/Cpp/expressions.mlir
index 9316d7b77619b..b07c5f1804d38 100644
--- a/mlir/test/Target/Cpp/expressions.mlir
+++ b/mlir/test/Target/Cpp/expressions.mlir
@@ -343,13 +343,13 @@ func.func @expression_with_subscript_user(%arg0: !emitc.ptr<!emitc.opaque<"void"
return %res_load : i32
}
-// CPP-DEFAULT: bool expression_with_load(int32_t [[VAL_1:v.+]], int32_t [[VAL_2:v.+]], int32_t* [[VAL_3:v.+]]) {
+// CPP-DEFAULT: bool expression_with_var_load_and_subscript(int32_t [[VAL_1:v.+]], int32_t [[VAL_2:v.+]], int32_t* [[VAL_3:v.+]]) {
// CPP-DEFAULT-NEXT: int64_t [[VAL_4:v.+]] = 0;
// CPP-DEFAULT-NEXT: int32_t [[VAL_5:v.+]] = 42;
// CPP-DEFAULT-NEXT: bool [[VAL_6:v.+]] = [[VAL_5]] + [[VAL_2]] < [[VAL_3]][[[VAL_4]]] + [[VAL_1]];
// CPP-DEFAULT-NEXT: return [[VAL_6]];
-// CPP-DECLTOP: bool expression_with_load(int32_t [[VAL_1:v.+]], int32_t [[VAL_2:v.+]], int32_t* [[VAL_3:v.+]]) {
+// CPP-DECLTOP: bool expression_with_var_load_and_subscript(int32_t [[VAL_1:v.+]], int32_t [[VAL_2:v.+]], int32_t* [[VAL_3:v.+]]) {
// CPP-DECLTOP-NEXT: int64_t [[VAL_4:v.+]];
// CPP-DECLTOP-NEXT: int32_t [[VAL_5:v.+]];
// CPP-DECLTOP-NEXT: bool [[VAL_6:v.+]];
@@ -358,7 +358,7 @@ func.func @expression_with_subscript_user(%arg0: !emitc.ptr<!emitc.opaque<"void"
// CPP-DECLTOP-NEXT: [[VAL_6]] = [[VAL_5]] + [[VAL_2]] < [[VAL_3]][[[VAL_4]]] + [[VAL_1]];
// CPP-DECLTOP-NEXT: return [[VAL_6]];
-func.func @expression_with_load(%arg0: i32, %arg1: i32, %arg2: !emitc.ptr<i32>) -> i1 {
+func.func @expression_with_var_load_and_subscript(%arg0: i32, %arg1: i32, %arg2: !emitc.ptr<i32>) -> i1 {
%c0 = "emitc.constant"() {value = 0 : i64} : () -> i64
%0 = "emitc.variable"() <{value = #emitc.opaque<"42">}> : () -> !emitc.lvalue<i32>
%ptr = emitc.subscript %arg2[%c0] : (!emitc.ptr<i32>, i64) -> !emitc.lvalue<i32>
@@ -373,6 +373,26 @@ func.func @expression_with_load(%arg0: i32, %arg1: i32, %arg2: !emitc.ptr<i32>)
return %result : i1
}
+// CPP-DEFAULT: bool expression_with_var_load(int32_t [[VAL_1:v.+]], int32_t [[VAL_2:v.+]]) {
+// CPP-DEFAULT-NEXT: int32_t [[VAL_3:v.+]] = 42;
+// CPP-DEFAULT-NEXT: return [[VAL_3]] + [[VAL_1]] < [[VAL_2]];
+
+// CPP-DECLTOP: bool expression_with_var_load(int32_t [[VAL_1:v.+]], int32_t [[VAL_2:v.+]]) {
+// CPP-DECLTOP-NEXT: int32_t [[VAL_3:v.+]];
+// CPP-DECLTOP-NEXT: [[VAL_3]] = 42;
+// CPP-DECLTOP-NEXT: return [[VAL_3]] + [[VAL_1]] < [[VAL_2]];
+
+func.func @expression_with_var_load(%arg0: i32, %arg1: i32) -> i1 {
+ %0 = "emitc.variable"() <{value = #emitc.opaque<"42">}> : () -> !emitc.lvalue<i32>
+ %result = emitc.expression : i1 {
+ %a = emitc.load %0 : !emitc.lvalue<i32>
+ %b = emitc.add %a, %arg0 : (i32, i32) -> i32
+ %d = emitc.cmp lt, %b, %arg1 :(i32, i32) -> i1
+ yield %d : i1
+ }
+ return %result : i1
+}
+
// CPP-DEFAULT: bool expression_with_load_and_call(int32_t* [[VAL_1:v.+]]) {
// CPP-DEFAULT-NEXT: int64_t [[VAL_2:v.+]] = 0;
// CPP-DEFAULT-NEXT: bool [[VAL_3:v.+]] = [[VAL_1]][[[VAL_2]]] + bar([[VAL_1]][[[VAL_2]]]) < [[VAL_1]][[[VAL_2]]];
|
|
@llvm/pr-subscribers-mlir Author: Kirill Chibisov (kchibisov) ChangesThe load side effect semantic depends on the operation that was used to declare the load's operand. In case of -- Not sure whether the logic of #144990 should be applied here, since the Full diff: https://github.com/llvm/llvm-project/pull/145011.diff 4 Files Affected:
diff --git a/mlir/include/mlir/Dialect/EmitC/IR/EmitC.td b/mlir/include/mlir/Dialect/EmitC/IR/EmitC.td
index 9ecdb74f4d82e..d88407a4d6726 100644
--- a/mlir/include/mlir/Dialect/EmitC/IR/EmitC.td
+++ b/mlir/include/mlir/Dialect/EmitC/IR/EmitC.td
@@ -966,6 +966,10 @@ def EmitC_LoadOp : EmitC_Op<"load", [CExpressionInterface,
let results = (outs AnyType:$result);
let assemblyFormat = "$operand attr-dict `:` type($operand)";
+
+ let extraClassDeclaration = [{
+ bool hasSideEffects();
+ }];
}
def EmitC_MulOp : EmitC_BinaryOp<"mul", []> {
diff --git a/mlir/lib/Dialect/EmitC/IR/EmitC.cpp b/mlir/lib/Dialect/EmitC/IR/EmitC.cpp
index e602210c2dc6c..10e0d931e17fd 100644
--- a/mlir/lib/Dialect/EmitC/IR/EmitC.cpp
+++ b/mlir/lib/Dialect/EmitC/IR/EmitC.cpp
@@ -898,6 +898,20 @@ LogicalResult emitc::VariableOp::verify() {
return verifyInitializationAttribute(getOperation(), getValueAttr());
}
+//===----------------------------------------------------------------------===//
+// VariableOp
+//===----------------------------------------------------------------------===//
+
+bool emitc::LoadOp::hasSideEffects() {
+ auto *defOp = this->getOperand().getDefiningOp();
+ // The load side effect semantic depends on the operation that was used to
+ // declare the load's operand, `VariableOp` and `MemberOp` perform loading
+ // from the stack, thus we may consider them as not having side effect to
+ // make end code more readable by letting those loads to get inlined.
+ return !defOp ||
+ !(llvm::isa<VariableOp>(defOp) || llvm::isa<MemberOp>(defOp));
+}
+
//===----------------------------------------------------------------------===//
// YieldOp
//===----------------------------------------------------------------------===//
diff --git a/mlir/test/Dialect/EmitC/transforms.mlir b/mlir/test/Dialect/EmitC/transforms.mlir
index a38f396dad953..687ebe2cd3612 100644
--- a/mlir/test/Dialect/EmitC/transforms.mlir
+++ b/mlir/test/Dialect/EmitC/transforms.mlir
@@ -135,21 +135,18 @@ func.func @single_result_requirement() -> (i32, i32) {
// CHECK-SAME: %[[VAL_1:.*]]: !emitc.ptr<i32>) -> i1 {
// CHECK: %[[VAL_2:.*]] = "emitc.constant"() <{value = 0 : i64}> : () -> i64
// CHECK: %[[VAL_3:.*]] = "emitc.variable"() <{value = #emitc.opaque<"42">}> : () -> !emitc.lvalue<i32>
-// CHECK: %[[VAL_4:.*]] = emitc.expression : i32 {
-// CHECK: %[[VAL_5:.*]] = load %[[VAL_3]] : <i32>
-// CHECK: yield %[[VAL_5]] : i32
+// CHECK: %[[VAL_4:.*]] = emitc.subscript %[[VAL_1]]{{\[}}%[[VAL_2]]] : (!emitc.ptr<i32>, i64) -> !emitc.lvalue<i32>
+// CHECK: %[[VAL_5:.*]] = emitc.expression : i32 {
+// CHECK: %[[VAL_6:.*]] = load %[[VAL_4]] : <i32>
+// CHECK: yield %[[VAL_6]] : i32
// CHECK: }
-// CHECK: %[[VAL_6:.*]] = emitc.subscript %[[VAL_1]]{{\[}}%[[VAL_2]]] : (!emitc.ptr<i32>, i64) -> !emitc.lvalue<i32>
-// CHECK: %[[VAL_7:.*]] = emitc.expression : i32 {
-// CHECK: %[[VAL_8:.*]] = load %[[VAL_6]] : <i32>
-// CHECK: yield %[[VAL_8]] : i32
+// CHECK: %[[VAL_7:.*]] = emitc.expression : i1 {
+// CHECK: %[[VAL_8:.*]] = load %[[VAL_3]] : <i32>
+// CHECK: %[[VAL_9:.*]] = add %[[VAL_8]], %[[VAL_5]] : (i32, i32) -> i32
+// CHECK: %[[VAL_10:.*]] = cmp lt, %[[VAL_9]], %[[VAL_0]] : (i32, i32) -> i1
+// CHECK: yield %[[VAL_10]] : i1
// CHECK: }
-// CHECK: %[[VAL_9:.*]] = emitc.expression : i1 {
-// CHECK: %[[VAL_10:.*]] = add %[[VAL_4]], %[[VAL_7]] : (i32, i32) -> i32
-// CHECK: %[[VAL_11:.*]] = cmp lt, %[[VAL_10]], %[[VAL_0]] : (i32, i32) -> i1
-// CHECK: yield %[[VAL_11]] : i1
-// CHECK: }
-// CHECK: return %[[VAL_9]] : i1
+// CHECK: return %[[VAL_7]] : i1
// CHECK: }
diff --git a/mlir/test/Target/Cpp/expressions.mlir b/mlir/test/Target/Cpp/expressions.mlir
index 9316d7b77619b..b07c5f1804d38 100644
--- a/mlir/test/Target/Cpp/expressions.mlir
+++ b/mlir/test/Target/Cpp/expressions.mlir
@@ -343,13 +343,13 @@ func.func @expression_with_subscript_user(%arg0: !emitc.ptr<!emitc.opaque<"void"
return %res_load : i32
}
-// CPP-DEFAULT: bool expression_with_load(int32_t [[VAL_1:v.+]], int32_t [[VAL_2:v.+]], int32_t* [[VAL_3:v.+]]) {
+// CPP-DEFAULT: bool expression_with_var_load_and_subscript(int32_t [[VAL_1:v.+]], int32_t [[VAL_2:v.+]], int32_t* [[VAL_3:v.+]]) {
// CPP-DEFAULT-NEXT: int64_t [[VAL_4:v.+]] = 0;
// CPP-DEFAULT-NEXT: int32_t [[VAL_5:v.+]] = 42;
// CPP-DEFAULT-NEXT: bool [[VAL_6:v.+]] = [[VAL_5]] + [[VAL_2]] < [[VAL_3]][[[VAL_4]]] + [[VAL_1]];
// CPP-DEFAULT-NEXT: return [[VAL_6]];
-// CPP-DECLTOP: bool expression_with_load(int32_t [[VAL_1:v.+]], int32_t [[VAL_2:v.+]], int32_t* [[VAL_3:v.+]]) {
+// CPP-DECLTOP: bool expression_with_var_load_and_subscript(int32_t [[VAL_1:v.+]], int32_t [[VAL_2:v.+]], int32_t* [[VAL_3:v.+]]) {
// CPP-DECLTOP-NEXT: int64_t [[VAL_4:v.+]];
// CPP-DECLTOP-NEXT: int32_t [[VAL_5:v.+]];
// CPP-DECLTOP-NEXT: bool [[VAL_6:v.+]];
@@ -358,7 +358,7 @@ func.func @expression_with_subscript_user(%arg0: !emitc.ptr<!emitc.opaque<"void"
// CPP-DECLTOP-NEXT: [[VAL_6]] = [[VAL_5]] + [[VAL_2]] < [[VAL_3]][[[VAL_4]]] + [[VAL_1]];
// CPP-DECLTOP-NEXT: return [[VAL_6]];
-func.func @expression_with_load(%arg0: i32, %arg1: i32, %arg2: !emitc.ptr<i32>) -> i1 {
+func.func @expression_with_var_load_and_subscript(%arg0: i32, %arg1: i32, %arg2: !emitc.ptr<i32>) -> i1 {
%c0 = "emitc.constant"() {value = 0 : i64} : () -> i64
%0 = "emitc.variable"() <{value = #emitc.opaque<"42">}> : () -> !emitc.lvalue<i32>
%ptr = emitc.subscript %arg2[%c0] : (!emitc.ptr<i32>, i64) -> !emitc.lvalue<i32>
@@ -373,6 +373,26 @@ func.func @expression_with_load(%arg0: i32, %arg1: i32, %arg2: !emitc.ptr<i32>)
return %result : i1
}
+// CPP-DEFAULT: bool expression_with_var_load(int32_t [[VAL_1:v.+]], int32_t [[VAL_2:v.+]]) {
+// CPP-DEFAULT-NEXT: int32_t [[VAL_3:v.+]] = 42;
+// CPP-DEFAULT-NEXT: return [[VAL_3]] + [[VAL_1]] < [[VAL_2]];
+
+// CPP-DECLTOP: bool expression_with_var_load(int32_t [[VAL_1:v.+]], int32_t [[VAL_2:v.+]]) {
+// CPP-DECLTOP-NEXT: int32_t [[VAL_3:v.+]];
+// CPP-DECLTOP-NEXT: [[VAL_3]] = 42;
+// CPP-DECLTOP-NEXT: return [[VAL_3]] + [[VAL_1]] < [[VAL_2]];
+
+func.func @expression_with_var_load(%arg0: i32, %arg1: i32) -> i1 {
+ %0 = "emitc.variable"() <{value = #emitc.opaque<"42">}> : () -> !emitc.lvalue<i32>
+ %result = emitc.expression : i1 {
+ %a = emitc.load %0 : !emitc.lvalue<i32>
+ %b = emitc.add %a, %arg0 : (i32, i32) -> i32
+ %d = emitc.cmp lt, %b, %arg1 :(i32, i32) -> i1
+ yield %d : i1
+ }
+ return %result : i1
+}
+
// CPP-DEFAULT: bool expression_with_load_and_call(int32_t* [[VAL_1:v.+]]) {
// CPP-DEFAULT-NEXT: int64_t [[VAL_2:v.+]] = 0;
// CPP-DEFAULT-NEXT: bool [[VAL_3:v.+]] = [[VAL_1]][[[VAL_2]]] + bar([[VAL_1]][[[VAL_2]]]) < [[VAL_1]][[[VAL_2]]];
|
The load side effect semantic depends on the operation that was used to declare the load's operand. In case of `VariableOp` and `MemberOp` the operation storage is on the stack, thus can be considered to not have side effects compared to e.g. subscript operation.
kchibisov
force-pushed
the
emitc-relax-variable-load
branch
from
94d3902 to
d4ac0b6
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The load side effect semantic depends on the operation that was used to declare the load's operand. In case of
VariableOpandMemberOpthe operation storage is on the stack, thus can be considered to not have side effects compared to e.g. subscript operation.--
Not sure whether the logic of #144990 should be applied here, since the
variable/memberops are rather limited iirc. cc @jacquesguan.